-
Notifications
You must be signed in to change notification settings - Fork 19
feat: Support proper encoding/decoding of TX metadata #1159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Akhil Repala <[email protected]>
Signed-off-by: Akhil Repala <[email protected]>
Signed-off-by: Akhil Repala <[email protected]>
Signed-off-by: Akhil Repala <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need something like a TransactionMetadatumWrapper
type, which can be used in a Transaction
type and call the decode functions, similar to what you're doing in TransactionMetadataSet
.
) | ||
} else { | ||
t.Logf("Length mismatch: original length = %d, re-encoded length = %d", len(dataBytes), len(encoded)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to keep the check for the re-encoded CBOR being identical to the original decoded CBOR. If that's failing with this new code, then that should be addressed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I will revert back the changes here
Type() int | ||
Cbor() []byte | ||
Metadata() *cbor.LazyValue | ||
Metadata() TransactionMetadataSet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want to use TransactionMetadataSet
here, only TransactionMetadatum
. The Set
is used at the block level to return the metadatum for all TXs included in the block.
type TransactionMetadataSet map[uint64]TransactionMetadatum | ||
|
||
type TransactionMetadatum interface { | ||
TypeName() string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a isTransactionMetadatum()
function here, so that other types can't accidentally satisfy this interface.
TransactionBodies []ConwayTransactionBody | ||
TransactionWitnessSets []ConwayTransactionWitnessSet | ||
TransactionMetadataSet map[uint]*cbor.LazyValue | ||
TransactionMetadataSet map[uint]common.TransactionMetadataSet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be TransactionMetadataSet TransactionMetadataSet
. What you've got here will give you nested maps, which isn't what we want
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean TransactionMetadataSet.TransactionMetadataSet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Err, I meant TransactionMetadataSet common.TransactionMetadataSet
Closes #1136